Skip to content

Document external data table paths in various contexts#584

Open
popescu-v wants to merge 1 commit into
mainfrom
560-specifying-additional-tables-problem-with-reference-table-name
Open

Document external data table paths in various contexts#584
popescu-v wants to merge 1 commit into
mainfrom
560-specifying-additional-tables-problem-with-reference-table-name

Conversation

@popescu-v

@popescu-v popescu-v commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator
  • In train_predictor, SNB_ or Baseline_-prefixed model dictionaries are generated. Similarly, in train_recoder, R_-prefixed model dictionaries are generated.

  • In deploy_model, these prefixed dictionaries must be used, even for external tables.

  • However, in evaluate_predictor, the original names must be used for external tables, in order to support evaluating several predictors.

Put your message here


TODO Before Asking for a Review

  • Rebase your branch to the latest version of main (or main-v10)
  • Make sure all CI workflows are green
  • When adding a public feature/fix: Update the Unreleased section of CHANGELOG.md (no date)
  • Self-Review: Review "Files Changed" tab and fix any problems you find
  • API Docs (only if there are changes in docstrings, rst files or samples):
    • Check the docs build without warning: see the log of the API Docs workflow
    • Check that your changes render well in HTML: download the API Docs artifact and open index.html
    • If there are any problems it is faster to iterate by building locally the API Docs

@popescu-v popescu-v requested a review from marcboulle June 8, 2026 14:43
@popescu-v popescu-v self-assigned this Jun 8, 2026
@popescu-v popescu-v linked an issue Jun 8, 2026 that may be closed by this pull request

@marcboulle marcboulle left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Petites changement suggérés.
A discuter si besoin (le comportement de la GUI et de l'API ne sont pas tout à fait identiques).

Comment thread doc/multi_table_primer.rst Outdated
Comment thread khiops/core/api.py Outdated
@popescu-v popescu-v force-pushed the 560-specifying-additional-tables-problem-with-reference-table-name branch from cacb98d to bf01eee Compare June 16, 2026 12:57
@popescu-v popescu-v requested a review from marcboulle June 16, 2026 12:58

@marcboulle marcboulle left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

- In `train_predictor`, `SNB_` or `Baseline_`-prefixed model
  dictionaries are generated. Similarly, in `train_recoder`,
  `R_`-prefixed model dictionaries are generated.

- In `deploy_model`, these prefixed dictionaries must be used, even for
  external tables.

- However, in `evaluate_predictor`, the original names must be used for
  external tables. It is the same name as the one used for training the
  predictor.
@popescu-v popescu-v force-pushed the 560-specifying-additional-tables-problem-with-reference-table-name branch from bf01eee to bdb4f6c Compare June 16, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Specifying additional tables : Problem with reference table name

2 participants